Graduate lockfile from experimental to stable#1212
Graduate lockfile from experimental to stable#1212brooke-hamilton wants to merge 7 commits intodevcontainers:mainfrom
Conversation
Signed-off-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com>
Signed-off-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com>
Signed-off-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com>
Signed-off-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com>
chrmarti
left a comment
chrmarti
left a comment
There was a problem hiding this comment.
Great, thanks! Really just one question: The two experimental properties are no longer needed in code, it seems?
| @@ -195,6 +195,8 @@ export interface ContainerFeatureInternalParams { | |||
| platform: NodeJS.Platform; | |||
| experimentalLockfile?: boolean; | |||
There was a problem hiding this comment.
Are the two experimental properties still needed?
There was a problem hiding this comment.
Fixed, thank you!
| const newLockfileContentString = JSON.stringify(lockfile, null, 2) + '\n'; | ||
| const newLockfileContent = Buffer.from(newLockfileContentString); | ||
| if (params.experimentalFrozenLockfile && !oldLockfileContent) { | ||
| if ((params.frozenLockfile || params.experimentalFrozenLockfile) && !oldLockfileContent) { |
There was a problem hiding this comment.
I think the experimental flag is already folded into the new property?
There was a problem hiding this comment.
Fixed, plus other locations with redundant experimental declarations.
Signed-off-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com>
Signed-off-by: Brooke Hamilton <45323234+brooke-hamilton@users.noreply.github.com>
|
@chrmarti please look at my most recent commit that adds This could cause confusion for developers who run a test and see the lockfile changed, giving a false indication that their code change caused the lockfile to change. I found this when running tests unrelated to the lockfile tests. The downside is that intentionally adding a lockfile to these folders requires an explicit |
| if (experimentalLockfile) { | ||
| process.stderr.write('Warning: --experimental-lockfile is deprecated. Lockfiles are now enabled by default.\n'); | ||
| } | ||
| if (experimentalFrozenLockfile) { |
There was a problem hiding this comment.
Not specific to this PR, but we should refactor these to avoid the duplicate logic.
3 participants
This pull request graduates the lockfile feature from experimental to stable. Lockfiles are now generated by default during
buildandupcommands to ensure reproducible builds. New CLI flags (--no-lockfileand--frozen-lockfile) are introduced for more explicit control, while the old experimental flags are deprecated but still accepted with warnings.Lockfile Feature Graduation and CLI Changes:
.devcontainer-lock.json) are now generated by default onbuildandup, pinning feature versions for reproducibility. [1] [2]--no-lockfile(disables lockfile generation/verification) and--frozen-lockfile(enforces lockfile existence and immutability). Deprecated--experimental-lockfileand--experimental-frozen-lockfilewith warnings. [1] [2] [3] [4] [5] [6]Documentation and Tooling:
README.mdandCHANGELOG.mdto reflect lockfile graduation and new CLI behavior. [1] [2]docs/contributing-code.mdto clarify packaging requirements before running tests.This PR closes issue #1195
There are no spec changes in this PR, but there is a related proposal at devcontainers/spec#727